mpris: Hide notification when !CanPlay, instead of closing player

I have observed a client in the wild (Chromium again) set CanPlay to
false momentarily while it is loading the next song. Previously, the
code would close the player proxy in that case, meaning that after
playing one track, the MPRIS message would disappear and never come
back.

However, I think this use of CanPlay, while apparently not usual, is not
incorrect according to the spec. We should hide the message instead of
closing the player proxy.

https://gitlab.gnome.org/GNOME/gnome-shell/issues/1362

diff --git a/js/ui/mpris.js b/js/ui/mpris.js
index 240e99e5f299042506e37246536a41e4f4adcf94..1519e47526638d0ba81f89577c1938d4d8c0bb09 100644 (file)
--- a/js/ui/mpris.js
+++ b/js/ui/mpris.js
@@ -44,11 +44,19 @@ class MediaMessage extends MessageList.Message {
                 this._player.next();
             });
 
-        this._player.connect('changed', this._update.bind(this));
-        this._player.connect('closed', this.close.bind(this));
+        this._updateHandlerId =
+            this._player.connect('changed', this._update.bind(this));
+        this._closedHandlerId =
+            this._player.connect('closed', this.close.bind(this));
         this._update();
     }
 
+    _onDestroy() {
+        super._onDestroy();
+        this._player.disconnect(this._updateHandlerId);
+        this._player.disconnect(this._closedHandlerId);
+    }
+
     vfunc_clicked() {
         this._player.raise();
         Main.panel.closeCalendar();
@@ -217,7 +225,7 @@ var MprisPlayer = class MprisPlayer {
             if (visible)
                 this.emit('show');
             else
-                this._close();
+                this.emit('hide');
         }
     }
 };
@@ -249,11 +257,14 @@ class MediaSection extends MessageList.MessageListSection {
             () => {
                 this._players.delete(busName);
             });
-        player.connect('show',
-            () => {
-                let message = new MediaMessage(player);
-                this.addMessage(message, true);
-            });
+        player.connect('show', () => {
+            this._message = new MediaMessage(player);
+            this.addMessage(this._message, true);
+        });
+        player.connect('hide', () => {
+            this.removeMessage(this._message, true);
+            this._message = null;
+        });
         this._players.set(busName, player);
     }
 
@@ -274,7 +285,10 @@ class MediaSection extends MessageList.MessageListSection {
         if (!name.startsWith(MPRIS_PLAYER_PREFIX))
             return;
 
-        if (newOwner && !oldOwner)
+        if (newOwner && !oldOwner) {
+            if (this._message)
+                this.removeMessage(this._message);
             this._addPlayer(name);
+        }
     }
 });